tools/pr-merge-conflict-fixer/publish.mts (1)

359-361: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Reading the whole patch just to check existence.

The buffer is discarded. Use statSync and, since the patch is sandbox-produced untrusted output, reject implausibly large files up front.

♻️ Proposed change

-  readFileSync(patchPath);
+  const patchStats = statSync(patchPath);
+  if (patchStats.size > 8 * 1024 * 1024) {
+    throw new ConflictFixerError("The resolution patch is unexpectedly large");
+  }

Update the import: import { mkdtempSync, rmSync, statSync } from "node:fs";

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tools/pr-merge-conflict-fixer/publish.mts` around lines 359 - 361, Replace
the discarded readFileSync call near artifactDirectory and patchPath with
statSync to check the patch exists without loading its contents. Validate the
resulting file size and reject implausibly large sandbox-produced patches before
continuing, and update the node:fs import accordingly.

test/pr-merge-conflict-fixer.test.ts (2)

35-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Isolate the fixture from ambient Git config for determinism.

merge.conflictStyle, core.autocrlf, hook paths, or init templates from a contributor's global config can change conflict output and flake these assertions.

♻️ Pin global/system config off

 function git(repository: string, args: string[]): string {
   return execFileSync("git", args, {
     cwd: repository,
     encoding: "utf8",
+    env: { ...process.env, GIT_CONFIG_GLOBAL: "/dev/null", GIT_CONFIG_SYSTEM: "/dev/null" },
     stdio: ["ignore", "pipe", "pipe"],
   }).trim();
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/pr-merge-conflict-fixer.test.ts` around lines 35 - 58, Update
createConflictFixture and its git helper setup to isolate the temporary
repository from ambient global and system Git configuration, disabling inherited
settings such as merge.conflictStyle, core.autocrlf, hooks, and init templates
while preserving the existing repository-specific configuration and fixture
behavior.

---

189-202: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for malformed patches and unresolved index entries.

The objectives call out malformed resolver output, but nothing exercises applyResolutionPatch rejection or the "leaves unresolved index entries" branch in validateResolutionPatch. Both are cheap to add with the existing fixture.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/pr-merge-conflict-fixer.test.ts` around lines 189 - 202, Add tests using
the existing conflict fixture to cover malformed resolver output rejected by
applyResolutionPatch and validateResolutionPatch rejecting patches that leave
unresolved index entries. Add focused cases alongside the existing
non-conflict-path test, asserting each path throws the expected validation error
while preserving current fixture setup.

tools/pr-merge-conflict-fixer/merge.mts (1)

33-56: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Two copies of the same Git exec helper, both using the default 1 MiB maxBuffer. cat-file blob and -z path listings can exceed that limit and throw ENOBUFS, aborting resolution or publication.

• tools/pr-merge-conflict-fixer/merge.mts#L33-L56: add an explicit maxBuffer to gitText/gitBuffer and export gitBuffer.
• tools/pr-merge-conflict-fixer/publish.mts#L120-L125: delete the local gitBuffer and import the shared one from merge.mts.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tools/pr-merge-conflict-fixer/merge.mts` around lines 33 - 56, The Git
helpers use the default buffer limit and are duplicated across modules. In
tools/pr-merge-conflict-fixer/merge.mts:33-56, add an explicit sufficiently
large maxBuffer to gitText and gitBuffer, and export gitBuffer; in
tools/pr-merge-conflict-fixer/publish.mts:120-125, remove the local gitBuffer
and import the shared helper from merge.mts.

tools/pr-merge-conflict-fixer/discover.mts (2)

132-150: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

One unfetchable PR head aborts the whole scan.

prepareMerge throws (via ensureCommit/clone failures) when a head SHA was force-pushed or deleted between listing and analysis. That exception propagates out of checkConflict and kills discovery for every remaining PR. Prefer logging and skipping the individual PR.

♻️ Skip the failing PR instead of aborting discovery

       try {
         const merge = prepareMerge(
           sourceRepository,
           mergeRepository,
           pullRequest.head.sha,
           baseSha,
         );
         return merge?.conflictPaths ?? null;
+      } catch (error) {
+        console.warn(
+          `Skipping PR #${pullRequest.number}: ${error instanceof Error ? error.message : String(error)}`,
+        );
+        return null;
       } finally {
         rmSync(temporaryDirectory, { force: true, recursive: true });
       }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tools/pr-merge-conflict-fixer/discover.mts` around lines 132 - 150, Update
the checkConflict callback passed to selectConflictingPullRequests so
prepareMerge failures for an individual pull request are caught, logged, and
skipped by returning null instead of propagating the exception. Preserve the
existing conflict-path result and temporary-directory cleanup, using the
available pull request context in the log.

---

103-122: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Bound the pagination loop and add a request timeout.

The loop has no upper page bound and fetch has no timeout, so a stalled or misbehaving API response can hang or spin the scan job indefinitely.

♻️ Proposed bound + timeout

-  for (let page = 1; ; page += 1) {
+  const maxPages = 50;
+  for (let page = 1; page <= maxPages; page += 1) {
     const response = await request(
       `https://api.github.com/repos/${repository}/pulls?state=open&base=main&per_page=100&page=${page}`,
       {
         headers: {
           Accept: "application/vnd.github+json",
           Authorization: `Bearer ${token}`,
           "X-GitHub-Api-Version": "2022-11-28",
         },
+        signal: AbortSignal.timeout(30_000),
       },
     );
     if (!response.ok) {
       throw new ConflictFixerError(
         `GitHub pull request listing failed with HTTP ${response.status}`,
       );
     }
     const pageItems = (await response.json()) as PullRequest[];
     pullRequests.push(...pageItems);
     if (pageItems.length < 100) return pullRequests;
   }
+  throw new ConflictFixerError("GitHub pull request listing exceeded the page limit");
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tools/pr-merge-conflict-fixer/discover.mts` around lines 103 - 122, Bound the
pagination loop in the pull-request discovery flow to a finite maximum page
count, and add an abort-based timeout to the request in the existing request
call. Preserve the current response validation and page accumulation behavior,
while ensuring stalled requests and non-terminating pagination exit through the
established error handling.

test/pr-merge-conflict-fixer-workflow-boundary.test.ts (1)

19-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Pipeline reports increased branching in this file (3 if statements, up from 0).

The Codebase Growth Guardrails check flagged this file. The record/checkout guard clauses could share one throwing helper to cut the branch count:

♻️ Proposed consolidation

+function required<T>(value: T | undefined, message: string): T {
+  if (!value) throw new Error(message);
+  return value;
+}
+
 function namedStep(job: Record<string, unknown>, name: string): Record<string, unknown> {
-  const step = steps(job).find((candidate) => candidate.name === name);
-  if (!step) throw new Error(`Missing workflow step: ${name}`);
-  return step;
+  return required(
+    steps(job).find((candidate) => candidate.name === name),
+    `Missing workflow step: ${name}`,
+  );
 }

 function checkout(job: Record<string, unknown>): Record<string, unknown> {
-  const step = steps(job).find((candidate) =>
-    String(candidate.uses ?? "").startsWith("actions/checkout@"),
-  );
-  if (!step) throw new Error("Missing checkout step");
-  return step;
+  return required(
+    steps(job).find((candidate) => String(candidate.uses ?? "").startsWith("actions/checkout@")),
+    "Missing checkout step",
+  );
 }

Verify against the repo's guardrail script whether this satisfies the threshold, or whether test helpers are meant to be exempted.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/pr-merge-conflict-fixer-workflow-boundary.test.ts` around lines 19 - 41,
Reduce branching in the test helpers by consolidating the throwing guard logic
used by record/checkout, while preserving their current fallback and error
behavior. Inspect the repository’s Codebase Growth Guardrails script to confirm
the required threshold and whether test helpers are exempt; only adjust these
helpers if needed to satisfy the guardrail.

Source: Pipeline failures

.github/workflows/pr-merge-conflict-fixer.yaml (1)

15-48: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Consider adding timeout-minutes to scan and publish.

Only resolve sets a job timeout (Line 48). scan and publish call the GitHub API (via discover.mts/publish.mts) and could hang up to the default 360-minute job limit if a network call stalls, tying up a runner unnecessarily.

Also applies to: 187-199

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/pr-merge-conflict-fixer.yaml around lines 15 - 48, Set
explicit timeout-minutes values on the scan and publish jobs, matching the
bounded execution policy already used by resolve. Add the timeouts directly to
the job definitions that run discover.mts and publish.mts, using an appropriate
limit that prevents stalled GitHub API calls from consuming the default runner
duration.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

test/pr-merge-conflict-fixer.test.ts (1)

50-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Duplicate required() helper across test files.

The same required<T> implementation appears verbatim in test/pr-merge-conflict-fixer-workflow-boundary.test.ts. Consider extracting it into a shared test helper (e.g. under test/helpers/) to avoid the two copies drifting apart.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/pr-merge-conflict-fixer.test.ts` around lines 50 - 55, Extract the
duplicated required<T> helper from pr-merge-conflict-fixer.test.ts and
pr-merge-conflict-fixer-workflow-boundary.test.ts into a shared test helper
under test/helpers, then import and reuse that single implementation in both
files while preserving its existing assertions and return behavior.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes